Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Saved filename fixed so that clients display it correctly #2415 #2705

Merged

Conversation

lama-buddy
Copy link
Contributor

@lama-buddy lama-buddy commented Sep 13, 2016

The fix:
The saved filename is changed to the following format: UUID_originalfilename

The names of files hosted in files.parse.com have the following format: UUID-originalfilename

The character that follows the UUID in the filenames (underscore and dash) is what allows the expandFilesInObject method to distinguish between Parse Server created/migrated files from files hosted on files.parse.com and generate the correct file URL.

// This makes file name property extraction for display in clients such as Parse Dashboard to work correctly.
// We can not use an UUID with dashes here because it is already used for files.parse.com hosted files and
// expandFilesInObject method below uses this distinction to construct the correct url for the file.
filename = randomHexString(36) + '_' + filename;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use a real UUID instead of a random hex? (32 chars + 4 dash right?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As explained in the comment, if we use a UUID, expandFilesInObject method will think the file is hosted in files.parse.com and construct the wrong URL. We want to keep the support for files.parse.com files and Parse Server created files so this keeps both working.

Copy link
Contributor

@flovilmart flovilmart Sep 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you look at the regex here https://github.com/ParsePlatform/parse-server/blob/e690b73bb5982cb8c8766e92c44e09f6dcd9aa99/src/Controllers/FilesController.js#L9

the separator between the UUID and the rest of the file is a - .

IN our case we use a _ for separating the random string and the filename. So that would still work

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true. So I can do something like this:

var hexstring = randomHexString(32)
var uuid = hexstring.substring(0,8) + "-" + hexstring.substring(8,12) + "-" + hexstring.substring(12,16) + "-" + hexstring.substring(16,20) + "-" + hexstring.substring(20);

filename = uuid + '_' + filename;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add the uuid package to generate a true UUID. (https://github.com/broofa/node-uuid)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. will do. thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks YOU!

@codecov-io
Copy link

codecov-io commented Sep 13, 2016

Current coverage is 92.21% (diff: 100%)

Merging #2705 into master will decrease coverage by <.01%

@@             master      #2705   diff @@
==========================================
  Files           102        102          
  Lines         12446      12447     +1   
  Methods        1559       1559          
  Messages          0          0          
  Branches       2039       2039          
==========================================
  Hits          11478      11478          
- Misses          968        969     +1   
  Partials          0          0          

Powered by Codecov. Last update f6312a1...e67039f

@ghost
Copy link

ghost commented Sep 13, 2016

@lama-buddy updated the pull request - view changes

@flovilmart flovilmart merged commit 22c1a87 into parse-community:master Sep 17, 2016
@lama-buddy lama-buddy deleted the extend-uploaded-filename-length branch September 19, 2016 01:47
yuki-takeichi added a commit to yuki-takeichi/parse-server that referenced this pull request Oct 6, 2016
@yuki-takeichi yuki-takeichi mentioned this pull request Oct 6, 2016
flovilmart pushed a commit that referenced this pull request Oct 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants